net: add support for finished after .destroy() by ronag · Pull Request #36635 · nodejs/node

ronag · 2020-12-26T13:42:02Z

Calling finished(socket, cb) would previously not
invoked the callback if the socket was already detroyed.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

ronag · 2020-12-26T13:42:18Z

@nodejs/net @nodejs/streams

lib/internal/streams/end-of-stream.js

mcollina

lgtm

nodejs-github-bot · 2020-12-26T15:17:20Z

CI: https://ci.nodejs.org/job/node-test-pull-request/35086/

test/parallel/test-net-finished.js

nodejs-github-bot · 2020-12-26T16:03:40Z

CI: https://ci.nodejs.org/job/node-test-pull-request/35087/ ✅

github-actions · 2020-12-28T05:44:45Z

Commit Queue failed

- Loading data for nodejs/node/pull/36635
✔  Done loading data for nodejs/node/pull/36635
----------------------------------- PR info ------------------------------------
Title      net: add support for finished after .destroy() (#36635)
Author     Robert Nagy  (@ronag)
Branch     ronag:net-finished -> nodejs:master
Labels     author ready, dont-land-on-v10.x, dont-land-on-v12.x, http, net, stream
Commits    4
 - net: add support for finished after .destroy()
 - fixup
 - Update lib/internal/streams/end-of-stream.js
 - Update test/parallel/test-net-finished.js
Committers 2
 - Robert Nagy 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/36635
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Rich Trott 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36635
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Rich Trott 
--------------------------------------------------------------------------------
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-12-26T16:03:40Z: https://ci.nodejs.org/job/node-test-pull-request/35087/
- Querying data for job/node-test-pull-request/35087/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Sat, 26 Dec 2020 13:42:02 GMT
   ✔  Approvals: 4
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/36635#pullrequestreview-558878340
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36635#pullrequestreview-558882856
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/36635#pullrequestreview-558893180
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36635#pullrequestreview-558935403
   ✖  This PR needs to wait 7 more hours to land
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/448230565

github-actions · 2020-12-30T17:59:32Z

Commit Queue failed

- Loading data for nodejs/node/pull/36635
✔  Done loading data for nodejs/node/pull/36635
----------------------------------- PR info ------------------------------------
Title      net: add support for finished after .destroy() (#36635)
Author     Robert Nagy  (@ronag)
Branch     ronag:net-finished -> nodejs:master
Labels     author ready, dont-land-on-v10.x, dont-land-on-v12.x, http, net, stream
Commits    1
 - net: add support for finished after .destroy()
Committers 1
 - Robert Nagy 
PR-URL: https://github.com/nodejs/node/pull/36635
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Rich Trott 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36635
Reviewed-By: Matteo Collina 
Reviewed-By: Luigi Pinca 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Rich Trott 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - net: add support for finished after .destroy()
   ✖  GitHub CI is still running
   ℹ  Last Full PR CI on 2020-12-30T15:28:27Z: https://ci.nodejs.org/job/node-test-pull-request/35087/
   ⚠  Commits were pushed after the last Full PR CI run:
   ⚠  - net: add support for finished after .destroy()
- Querying data for job/node-test-pull-request/35087/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Sat, 26 Dec 2020 13:42:02 GMT
   ✔  Approvals: 4
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/36635#pullrequestreview-558878340
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36635#pullrequestreview-558882856
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/36635#pullrequestreview-558893180
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36635#pullrequestreview-558935403
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/453124569

ronag · 2020-12-31T09:14:21Z

@Trott Would you mind help landing this? At the moment I am unable to land PR's and the bots don't seem to work.

nodejs-github-bot · 2021-01-03T18:04:15Z

CI: https://ci.nodejs.org/job/node-test-pull-request/35227/

jasnell · 2021-01-04T21:18:09Z

Landed in 2da3611

Calling `finished(socket, cb)` would previously not invoked the callback if the socket was already detroyed. PR-URL: #36635 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins · 2021-01-05T16:31:13Z

hey @ronag this doesn't land cleanly on v15.x. Would you be able to backport?

MylesBorins · 2021-04-04T19:25:20Z

Adding don't land labels for all current branches as this was reverted from master

ronag added net Issues and PRs related to the net subsystem. stream Issues and PRs related to the stream subsystem. labels Dec 26, 2020

ronag requested review from mafintosh and mcollina December 26, 2020 13:42

nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Dec 26, 2020

ronag requested a review from jasnell December 26, 2020 13:42

ronag added dont-land-on-v10.x labels Dec 26, 2020

RaisinTen reviewed Dec 26, 2020

View reviewed changes

lib/internal/streams/end-of-stream.js Outdated Show resolved Hide resolved

mcollina approved these changes Dec 26, 2020

View reviewed changes

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 26, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 26, 2020

RaisinTen reviewed Dec 26, 2020

View reviewed changes

test/parallel/test-net-finished.js Outdated Show resolved Hide resolved

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 26, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 26, 2020

ronag added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 26, 2020

lpinca approved these changes Dec 26, 2020

View reviewed changes

benjamingr approved these changes Dec 26, 2020

View reviewed changes

Trott approved these changes Dec 27, 2020

View reviewed changes

ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 28, 2020

github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 28, 2020

targos added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 28, 2020

ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 30, 2020

ronag force-pushed the net-finished branch from ef008d3 to 3ca99ea Compare December 30, 2020 17:15

github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 30, 2020

github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 30, 2020

ronag added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 30, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 31, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 31, 2020

jasnell approved these changes Dec 31, 2020

View reviewed changes

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2021

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 2, 2021

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021

jasnell closed this Jan 4, 2021

MylesBorins added the backport-blocked-v15.x label Jan 5, 2021

ronag self-assigned this Jan 5, 2021

mcollina mentioned this pull request Mar 29, 2021

http: significant performance regression on master #37937

Closed

3 tasks

MylesBorins added dont-land-on-v14.x and removed backport-blocked-v15.x labels Apr 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

net: add support for finished after .destroy()#36635

net: add support for finished after .destroy()#36635
ronag wants to merge 1 commit intonodejs:masterfrom
nxtedition:net-finished

ronag commented Dec 26, 2020 •

edited

Loading

Uh oh!

ronag commented Dec 26, 2020

Uh oh!

Uh oh!

mcollina left a comment

Uh oh!

nodejs-github-bot commented Dec 26, 2020

Uh oh!

Uh oh!

nodejs-github-bot commented Dec 26, 2020 •

edited by ronag

Loading

Uh oh!

github-actions bot commented Dec 28, 2020

Uh oh!

github-actions bot commented Dec 30, 2020

Uh oh!

ronag commented Dec 31, 2020

Uh oh!

nodejs-github-bot commented Jan 3, 2021

Uh oh!

jasnell commented Jan 4, 2021

Uh oh!

MylesBorins commented Jan 5, 2021

Uh oh!

MylesBorins commented Apr 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

Uh oh!

Conversation

ronag commented Dec 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

ronag commented Dec 26, 2020

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 26, 2020

Uh oh!

Uh oh!

nodejs-github-bot commented Dec 26, 2020 • edited by ronag Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 28, 2020

Uh oh!

github-actions bot commented Dec 30, 2020

Uh oh!

ronag commented Dec 31, 2020

Uh oh!

nodejs-github-bot commented Jan 3, 2021

Uh oh!

jasnell commented Jan 4, 2021

Uh oh!

MylesBorins commented Jan 5, 2021

Uh oh!

MylesBorins commented Apr 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

ronag commented Dec 26, 2020 •

edited

Loading

nodejs-github-bot commented Dec 26, 2020 •

edited by ronag

Loading